Skip to content

fix(remote-workspace): observe Pi prompt failures before acknowledgement - #761

Closed
luvs01 wants to merge 8 commits into
devfrom
fix/pi-prompt-rejection-lifetime
Closed

luvs01 wants to merge 8 commits into
devfrom
fix/pi-prompt-rejection-lifetime

Conversation

@luvs01

@luvs01 luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

A Pi child can fail while its prompt acknowledgement is still pending. The runtime created a separate turn-settlement promise but did not observe it until after awaiting that acknowledgement. A process exit, malformed RPC output, or early extension error could therefore reject both promises while only the command rejection was handled. Even a caller that caught handle.prompt() still received a process-level unhandledRejection.

Observe prompt admission and turn settlement concurrently with Promise.all. Either failure rejects the prompt immediately, and success still requires both a successful prompt response and the turn's settlement event. Clear only the active settlement owned by this prompt. Process launch, remote permissions, tool admission, cleanup and replay policy are unchanged.

Add seven isolated synthetic RPC scenarios covering child exit, malformed output, extension errors before/after acknowledgement, explicit rejection, and successful completion in either order. The fixture records process-level unhandled rejections without changing the test runner's rejection policy. Update the architecture contract and English/Korean guide.

This is a pre-existing application lifetime bug reproduced on official Bun 1.4.3 and 1.4.4, not a newly introduced Bun 1.4.4 regression. The daemon's crash guard may absorb an unhandled rejection; this PR does not claim that every occurrence terminates the daemon.

Verification

Base: fork dev at c4d4bb2da2162e23c314befafc5ec104292902a0. Local execution: Linux x64. Tests use the repository's normal preload/shared lock in the host PID namespace.

Reproduction and regression

On untouched base, a real synthetic child exiting with code 7 before acknowledging a prompt produced both:

  • caught prompt error: Pi RPC exited with code 7;
  • one unhandled rejection with the same message.

This was reproduced on both Bun versions. Early extension failure and malformed RPC output independently produced the same double-reporting shape on Bun 1.4.4. With this patch, the prompt still rejects with the relevant error and the fixture records zero unhandled rejections.

The fixture accepts an absolute source path, so the exact same probe can target a baseline worktree:

bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts exit-before-ack
bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts extension-before-ack
bun tests/fixtures/pi-prompt-lifetime.ts /absolute/base/src/remote-control/workspace-pi-runtime.ts malformed-before-ack

Commands and results

Command Result
bun run test --parallel=1 ./tests/clients/remote-workspace-cli-runtimes.test.ts — pinned Bun 1.4.3 8 passed, 1 opt-in real-Pi skip, 0 failed; 22 assertions
bun test ./tests/clients/remote-workspace-cli-runtimes.test.ts — official Bun 1.4.4 8 passed, 1 opt-in real-Pi skip, 0 failed; 22 assertions
bun run test:changed --parallel=2 ./tests/clients/ ./tests/cli/ ./tests/ci-workflows/docs-link-targets.test.ts ./tests/ci-workflows/file-size-ratchet.test.ts — pinned Bun 1.4.3 1,396 passed, 33 opt-in/platform skips, 0 failed; 8,040 assertions across 60 files
bun run test --parallel=2 ./tests/ci-workflows/file-size-ratchet.test.ts ./tests/ci-workflows/docs-link-targets.test.ts ./tests/clients/remote-workspace-sessions.test.ts ./tests/clients/remote-workspace-management.test.ts — explicit source-reading and session oracles 50 passed, 0 failed; 247 assertions across 4 files
bun run typecheck; bun run structure:check; bun run privacy:scan; git diff --check --cached Passed
In docs-site/: bun install --frozen-lockfile --offline; OPENCODEX_DOCS_OFFLINE=1 bun run build Frozen install unchanged; 593 pages built, 84,861 internal links checked

The focused subprocess fixture is explicitly invoked because changed-mode import analysis cannot follow the source filename passed to a child. The broad repository suite was not rerun under the documented constrained-worktree/resource exception; validation covers the changed runtime and connected client/CLI paths plus explicit session, documentation and size guards. Exact-head hosted CI remains separate. Native Windows/macOS execution, real Pi/provider execution, and live CLI confinement are unverified; opt-in skips are not counted as passing evidence.

Independent source/test/documentation review by a separate agent found no material issue. This is an agent review, not a maintainer approval or hosted bot review.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The patch changes promise observation only; it does not change credentials, process argv, approved roots, remote capabilities, tool execution, or automatic replay. Privacy scan passed.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes

    • Remote workspace sessions now report prompt-start failures promptly if Pi exits or an extension encounters an error, without waiting for a turn acknowledgment.
    • A prompt is considered successful only after both Pi accepts it and the turn settles, helping prevent failures from going unreported.
  • Documentation

    • Updated remote workspace guidance in English and Korean to explain how prompt failures are reported.

github-actions Bot and others added 2 commits October 23, 2026 23:42
…#7633)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Observe prompt admission and turn settlement concurrently so early child failures cannot escape as unhandled rejections.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between c4d4bb2 and 1d56a1d.

📒 Files selected for processing: 6
  • docs-site/src/content/docs/guides/remote-workspace.md
  • docs-site/src/content/docs/ko/guides/remote-workspace.md
  • src/remote-control/workspace-pi-runtime.ts
  • structure/remote-workspace.md
  • tests/clients/remote-workspace-cli-runtimes.test.ts
  • tests/fixtures/pi-prompt-lifetime.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

PiRpcProcess.prompt now observes prompt acceptance and turn settlement concurrently. Either operation can reject the prompt without waiting for the other. Tests and documentation cover this behavior.

Changes: Pi prompt lifetime

Layer / File(s) Summary
Concurrent prompt observation and validation
src/remote-control/workspace-pi-runtime.ts, tests/fixtures/pi-prompt-lifetime.ts, tests/clients/remote-workspace-cli-runtimes.test.ts, docs-site/src/content/docs/guides/remote-workspace.md, docs-site/src/content/docs/ko/guides/remote-workspace.md, structure/remote-workspace.md
prompt observes acceptance and settlement concurrently, and clears activeSettle in finally only when it still matches the prompt. The subprocess fixture and parameterized test cover seven RPC modes and check for unhandled errors. The documentation describes failure reporting before Pi acknowledges a turn.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: lidge-jun, ingwannu

Merge Risk: Minimal · up to 1d56a

The changed tests cover the reported early-failure regression. No actionable issue currently blocks merging after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: observing Pi prompt failures before acknowledgement in the remote-workspace runtime.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 23, 2026
@luvs01

luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner Author

No description provided.

lidge-jun and others added 6 commits October 24, 2026 07:54
…jun#7642)

GHSA-mrhx-c7xc-m3w5 and GHSA-27r3-jxmh-p828 blocked the 2.92.0 publish.
Leave the package version at 2.93.0 on dev.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…e-jun#7644)

Co-authored-by: lidge-jun <243035832+lidge-jun@users.noreply.github.com>
…idge-jun#7528)

Settle async Windows ACL work and native-main owner locks before fixture removal, so hosted-Windows teardowns no longer fail with EPERM after passing assertions. No retry, sleep or timeout changes.

Refs lidge-jun#7515
…deterministic (lidge-jun#7552)

Make the codex-runtime, client-connect and loopback-listener tests deterministic: bounded child deadlines with process milestones, and a port-0 public listener in place of the free-port-then-bind race. No sleeps or timeout increases.

Refs lidge-jun#7403 lidge-jun#7404

Co-authored-by: Shreyas652 <66720396+Shreyas652@users.noreply.github.com>
lidge-jun#7598)

Keep the cold first-exec of a fresh Bun copy and the PowerShell start out of two timed cases: the PATH-fallback cases share one warmed Bun copy created in a budgeted beforeAll, and the CurrentUser Root certificate inspection is replayed once before its timed case. The 10s product deadline is unchanged; a new seam test drives a deadline breach to unknown on every platform.

Refs lidge-jun#7578 lidge-jun#7569
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

⏳ DRAFT

  • UI screenshot required.

What to do

  • Add a screenshot of the UI change to the PR description.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions
github-actions Bot marked this pull request as draft October 24, 2026 08:13

luvs01 commented Oct 24, 2026

Copy link
Copy Markdown
Owner Author

Closing this tracking PR after carrying the validated change into the upstream review queue. Commit 13acca5ae4a8d869eda64b2918758702ac39385d preserves all six change-owned file blobs from 1d56a1df7fee7a793466b7dd0d9cb17607ba75ed and integrates the current dev security dependencies. Native Windows Bun 1.4.4 validation passed 8 cases with one optional real-Pi skip, and the scoped static checks passed. The branch is preserved; this closure does not mean the change has merged or shipped.

@luvs01 luvs01 closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants